Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

Conversation

@tbarlow12
Copy link
Contributor

@tbarlow12 tbarlow12 commented May 24, 2019

  • Refactored Azure login logic into AzureLoginService
  • Added helper util for tests (invokeHook)
  • Added more functionality to Mock Factory
  • All tested plugins have 100% test coverage

@tbarlow12 tbarlow12 requested review from PIC123, mydiemho and pjlittle May 24, 2019 20:21
@tbarlow12 tbarlow12 changed the title test: Unit tests for deploy and login plugins test: Unit tests for deploy, login, package, delete and APIM plugins May 24, 2019
Copy link
Contributor

@PIC123 PIC123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall

@tbarlow12 tbarlow12 merged commit 2b6575b into dev May 24, 2019
Copy link
Contributor

@mydiemho mydiemho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marvelous job! have some questions about some changes, let me know if any doesn't make sense

catch (e) {
this.serverless.cli.log('Error logging into azure');
this.serverless.cli.log(e);
this.serverless.cli.log(`${e}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should throw error and short circuit when login failed.

BindingUtils.getBindingsMetaData(sls);
expect(sls.cli.log).toBeCalledWith('Parsing Azure Functions Bindings.json...');
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we testing other methods in a different PR?

@@ -0,0 +1,24 @@
export const constants = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.js also export constants, is this gonna clash?


describe('Login Plugin', () => {

const authResponse = MockFactory.createTestAuthResponse();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of duplicate code for setting up AzureLoginService. can we move some of them to a beforeEach?

@tbarlow12 tbarlow12 deleted the tabarlow/deploy-unit-tests branch May 28, 2019 13:23
tbarlow12 added a commit that referenced this pull request Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants